Conversation
There was a problem hiding this comment.
Pull request overview
This pull request addresses ID mismatch issues when converting old COLMAP databases to GLOMAP format. The fix ensures that when frame_id or rig_id don't exist in the database, they are assigned values consistent with image_id or camera_id respectively, rather than being randomly incremented.
Changes:
- Modified rig ID assignment logic to use camera_id when no existing rigs are present
- Modified frame ID assignment logic to use image_id when no existing frames are present
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
glomap/io/colmap_converter.cc
Outdated
| if (cameras_id_to_rig_id.find(camera_id) == cameras_id_to_rig_id.end()) { | ||
| Rig rig; | ||
| rig.SetRigId(++max_rig_id); | ||
| if (max_rig_id != 0) |
There was a problem hiding this comment.
ID collision bug: When max_rig_id equals 0 but a rig with ID 0 exists in the database, using camera_id directly can cause a collision. For example, if there's an existing rig with ID 0, and a new camera has camera_id=0, this will create a duplicate rig ID. The condition should check whether any rigs exist, not just whether max_rig_id is non-zero. Consider using rigs.empty() or always incrementing from max_rig_id.
| if (max_rig_id != 0) | |
| // If any rigs already exist, assign a new unique rig ID by incrementing | |
| // the current maximum. Otherwise, for the very first rig, reuse the | |
| // camera_id as the rig ID to preserve original behavior. | |
| if (!rigs.empty()) |
glomap/io/colmap_converter.cc
Outdated
| for (auto& [image_id, image] : images) { | ||
| if (image.frame_id == colmap::kInvalidFrameId) { | ||
| frame_t frame_id = ++max_frame_id; | ||
| frame_t frame_id = (max_frame_id > 0)? ++max_frame_id : image_id; |
There was a problem hiding this comment.
ID collision bug: When max_frame_id equals 0 but a frame with ID 0 exists in the database, using image_id directly can cause a collision. For example, if there's an existing frame with ID 0, and an image has image_id=0, this will create a duplicate frame ID. Consider checking frames.empty() or always incrementing from max_frame_id to avoid collisions.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
2 participants
A heuristic fix for the id mismatch for the old colmap database. Previously, frame_id / rig_id are randomly assigned when they do not exist in database, now set it to be consistent with the image_id / camera_id.